add top_hit metric by scampi · Pull Request #7302 · elastic/kibana

scampi · 2016-05-26T22:41:47Z

Display the first/last value of a field, for a set of documents sorted on some field (e.g., a date).

TODO

aggregation of multivalued field
object in array use case
use AggConfigResult.toString for displaying arrays in the metric vis

Test

Index

Create the weather index with the following mapping:

$ curl -XPUT 'http://localhost:9200/weather' -d '
{
   "mappings": {
      "Weather": {
         "properties": {
            "timestamp": {
               "type": "date",
               "format": "date_optional_time"
            },
            "location": {
               "index": "not_analyzed",
               "type": "string"
            }
         }
      }
   }
}
'

Data

Add a document at a time.

$ curl -XPOST 'http://localhost:9200/weather/Weather/' -d '
{
  "location": "France",
  "temperature": 28,
  "timestamp": "2016-05-26T21:22:18+01:00"
}
'
$ curl -XPOST 'http://localhost:9200/weather/Weather/' -d '
{
  "location": "France",
  "temperature": 23.5,
  "timestamp": "2016-05-26T22:59:04+01:00"
}
'

Visualization

We can see what is the latest recorded temperature in France by doing the following:

Create either a vertical bar or a data table:

add a Latest metric:
- "Field": the field which value to display, here temperature
- "Sort On": the field to sort the data on, here timestamp
add a terms bucket on the field location.

ghost · 2016-05-26T22:41:49Z

Jenkins standing by to test this. If you aren't a maintainer, you can ignore this comment. Someone with commit access, please review this and clear it for Jenkins to run; then say 'jenkins, test it'.

panda01 · 2016-06-09T20:16:53Z

Jenkins, test it.

scampi · 2016-06-09T20:28:49Z

In the jenkins logs, it fails with the exception java.lang.IllegalArgumentException: unknown setting [es.path.conf] did you mean [path.conf]?. Do you know what that is ?

scampi · 2016-06-09T20:54:03Z

With this fix elastic/libesvm@e3fd2ca I am able to run tests on my machine. Thanks @spalger for the fix ;o)

spalger · 2016-06-09T21:10:56Z

jenkins, test it

scampi · 2016-06-10T08:51:39Z

jenkins, test it

panda01 · 2016-06-10T18:17:10Z

Only, we (elastic members) can trigger the tests.

panda01 · 2016-06-10T18:17:20Z

Jenkins, test it.

panda01 · 2016-06-10T18:32:53Z

So it seems that this doesn't work with nested fields, through (I believe) no fault of your own, I'm not really sure where to go to fix this either.

panda01 · 2016-06-10T18:33:38Z

Some other images that helped me reach this conclusion.

panda01 · 2016-06-10T18:35:58Z

Also, I'm not a fan this implementation, it is IMO is a bit limiting. I feel like the top_hits agg has other uses and by you only limiting it to order: desc it becomes a latest.

I say we take this away and leave it as kind of a top_hits agg. As an extra we can add if you want to do latest here is how you do it.

panda01 · 2016-06-10T18:40:24Z

Hmm, also I don't think these tests are something you caused. By the time you come back around they should be good to go though.

scampi · 2016-06-11T14:55:17Z

Thanks for spotting the problem with nested fields.

As an extra we can add if you want to do latest here is how you do it.

I think there is something missing to your comment, since I don't understand that sentence as it is now.

panda01 · 2016-06-13T13:39:59Z

@scampi Nope, I just worded that poorly. What I mean to say is after you add this metric, and remove the limitation on order, you can add steps that people can follow in order to get the latest value.

scampi · 2016-06-14T07:24:19Z

@panda01 Sorry I am still confused...

Do you want me to provide a combobox with, e.g., two options (desc and asc), so that the order option is not hardcoded ?

Otherwise, can you clarify what those steps are ? For the issue #6877 I need to either sort by descending (or ascending for some use case?) order. From what I read in the sort options, the default sort does not do what is needed for this issue.

panda01 · 2016-06-14T14:07:34Z

Do you want me to provide a combobox with, e.g., two options (desc and asc), so that the order option is not hardcoded ?

Exactly this! Maybe radio boxes though since i hate select boxes.

scampi · 2016-06-14T21:13:35Z

@panda01 I added a select box for choosing the order instead of radio boxes. I thought it was best to be consistent with the rest of kibana, e.g., the terms bucket aggregation uses a select box for the order. I can revert though.

In addition, the sort field is now by default the time field associated to the index pattern, as I had forgotten it mentioned in the issue.

scampi · 2016-06-15T07:39:01Z

@panda01 there is anyway another issue about the select box #2130 (comment) where the cool ui-select is mentioned ;o)

panda01 · 2016-06-21T12:50:16Z

@scampi in regards to using chosen, I would say the normal select box is appropriate for now. I will check out your changes.

panda01 · 2016-06-21T12:54:38Z

src/ui/public/agg_types/controls/latest.html

Hmm, this is a bit nitpicky of me, but I think the sort, and sort order should use the same property names, since they are dependent on one another, and I would hate for someone later to not realize that.

For instance you could call the sort field agg.params.sort.field and the order agg.params.sort.dir or something to that effect. but with that it's clear they're both dealing with the sort, no matter where it's put.

…f there is only one

scampi · 2016-12-28T17:15:53Z

@Bargs I updated the aggregate menu. Regarding the sorting, I don't see what is wrong. Can you explain ? I tried to take the last/first value of a simple index and it was fine.

Bargs

I was having an issue where flipping between ascending and descending sort was giving me different values every time, as if the document was getting picked randomly. I see now that I accidentally screenshotted the response and not the request, so that's not helpful. Unfortunately I'm not sure what was happening and I can't reproduce it now. I'll let you know if I run into it again.

Otherwise, LGTM! All yours, @ppisljar

ppisljar · 2016-12-30T07:05:09Z

i am not sure whats happening ... i am using logstash data. if i do top hits on the bytes field (with default size of 1) it works ok ... if i try the same with memory fields it only returns - .. however if i increase size to lets say 2 then it works with the memory field as well ?

ppisljar · 2016-12-30T07:18:34Z

another visualization type was added (heatmap) ... could you also add the top hits there ... it should behave the same as with other xy charts (line,bar,area)

ppisljar · 2016-12-30T11:10:05Z

This looks really good and its great to see all the way this came (it was marked as low fruit hehe :) ). Besides the two small things mentioned above i think this is good to go.

ppisljar · 2016-12-30T13:41:01Z

try running npm run lintroller it seems to find two lets which should be consts (and auto updates them ... so you just need to commit after)

…ned values

scampi · 2017-01-03T11:40:51Z

@ppisljar Regarding #7302 (comment) it seems that some documents don't have the memory field from what I saw locally. In your case, it would mean that the last before last has that field, but not the last one.

The last commits add the following:

added top hits metric to the heatmap.
fixed min/max of an array of values: it was returning infinity if all values were undefined/null.

Regarding that 2nd point, I would like to know if there is some default "zero" value. For example, if a field is missing:

if a string, then we'd return "N/A" or something set in the advanced settings
if a number, then we'd return "0" or "1" or something set in the advanced settings

The PR is now ready for review, but pending the answer to the previous question, some changes might be needed.

ppisljar · 2017-01-03T13:18:26Z

regarding second point it should probably behave the same as for non-array missing values ? @Bargs whats your opinion ?

Bargs · 2017-01-03T16:33:21Z

Discover displays a - for any missing fields regardless of type, so I think we should do the same here to remain consistent.

scampi · 2017-01-03T17:10:58Z

ok then PR is ready ;o) I thought there was such a setting so I wanted to make sure

ppisljar · 2017-01-04T21:48:12Z

congrats scampi, this is a great PR and i am sure many are excited about finally having top hits in kibana!

ppisljar · 2017-01-04T21:51:29Z

the backport #9735 has some conflicts you'll need to resolve.

Bargs · 2017-01-04T22:39:07Z

@scampi thanks for the great work on another giant PR! This is an awesome new metric, users are gonna love it.

scampi · 2017-01-05T09:37:30Z

Thanks @Bargs and @ppisljar for your help on this ;o)

scampi changed the title ~~add top_hits aggregation~~ add top_hits metric May 26, 2016

panda01 added release_note:enhancement good first issue low hanging fruit P2 labels Jun 9, 2016

panda01 self-assigned this Jun 9, 2016

scampi force-pushed the latest-value branch from d8a7c60 to 07b11cd Compare June 10, 2016 08:11

panda01 added the needs updates label Jun 10, 2016

scampi force-pushed the latest-value branch from 07b11cd to 7f1def9 Compare June 11, 2016 14:57

epixa removed the P2 label Jun 13, 2016

panda01 reviewed Jun 21, 2016
View reviewed changes

Bargs added needs updates and removed review labels Dec 22, 2016

filter out incompatble aggregate options and auto select the option i…

1bd4727

…f there is only one

Bargs approved these changes Dec 28, 2016

View reviewed changes

Bargs added review and removed needs updates labels Dec 28, 2016

simplified condition

5babf9a

scampi added 4 commits December 30, 2016 19:43

lint

7372fce

Merge branch 'master' of github.com:elastic/kibana into latest-value

67088c8

fix min/max/sum/average aggregate of elements having only null/undefi…

0791be7

…ned values

added top_hits metric to heatmap visualization

c967a23

ppisljar approved these changes Jan 4, 2017

View reviewed changes

elastic-jasper mentioned this pull request Jan 4, 2017

[5.x] add top_hit metric #9735

Closed

This was referenced Jan 6, 2017

add top_hit metric #9759

Closed

[5.x] add top_hit metric #9809

Merged

Bargs mentioned this pull request Mar 16, 2017

Panel that shows the latest value of a field #678

Closed

spalger mentioned this pull request Jul 3, 2018

"Top Hit" aggregation not listed in docs #20427

Closed

Conversation

scampi commented May 26, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

TODO

Test

Index

Data

Visualization

Uh oh!

ghost commented May 26, 2016

Uh oh!

panda01 commented Jun 9, 2016

Uh oh!

scampi commented Jun 9, 2016

Uh oh!

scampi commented Jun 9, 2016

Uh oh!

spalger commented Jun 9, 2016

Uh oh!

scampi commented Jun 10, 2016

Uh oh!

panda01 commented Jun 10, 2016

Uh oh!

panda01 commented Jun 10, 2016

Uh oh!

panda01 commented Jun 10, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

panda01 commented Jun 10, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

panda01 commented Jun 10, 2016

Uh oh!

panda01 commented Jun 10, 2016

Uh oh!

scampi commented Jun 11, 2016

Uh oh!

panda01 commented Jun 13, 2016

Uh oh!

scampi commented Jun 14, 2016

Uh oh!

panda01 commented Jun 14, 2016

Uh oh!

scampi commented Jun 14, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

scampi commented Jun 15, 2016

Uh oh!

panda01 commented Jun 21, 2016

Uh oh!

panda01 Jun 21, 2016

Choose a reason for hiding this comment

Uh oh!

scampi commented Dec 28, 2016

Uh oh!

Bargs left a comment

Choose a reason for hiding this comment

Uh oh!

ppisljar commented Dec 30, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ppisljar commented Dec 30, 2016

Uh oh!

ppisljar commented Dec 30, 2016

Uh oh!

ppisljar commented Dec 30, 2016

Uh oh!

scampi commented Jan 3, 2017

Uh oh!

ppisljar commented Jan 3, 2017

Uh oh!

Bargs commented Jan 3, 2017

Uh oh!

scampi commented Jan 3, 2017

Uh oh!

ppisljar commented Jan 4, 2017

Uh oh!

ppisljar commented Jan 4, 2017

Uh oh!

Bargs commented Jan 4, 2017

scampi commented May 26, 2016 •

edited

Loading

panda01 commented Jun 10, 2016 •

edited

Loading

panda01 commented Jun 10, 2016 •

edited

Loading

scampi commented Jun 14, 2016 •

edited

Loading

ppisljar commented Dec 30, 2016 •

edited

Loading